Skip to content

Conversation

galvez
Copy link
Contributor

@galvez galvez commented Jul 28, 2025

This adds a browser error handler and associated error formatting.

Screenshot 2025-08-03 at 19 24 42

There's on unsolved issue though: when accessing transformResult, it seems map is always null.

Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks.

  • communication
    • I think we can communicate via the HotChannel (e.g. transport.send({ type: 'clientError' })) instead of a fetch request. This way we can re-use the same code when we send error information from other environments.
  • error collection
    • I think we should listen the unhandledrejection event as well to catch the unhandled promise rejections.
  • codeframe generation
    • generateCodeFrame function can be used for the code frame generation.
    • fetch(info.filename) can be probably replaced with moduleGraph.getModuleByUrl(id).transformResult
    • [optional] It seems colno and lineno contains the position for the transformed script. I think we need to map that back to the original code. That should be possible by a code similar to
      const mod = moduleGraph.getModuleById(id)
      const rawSourceMap = mod?.transformResult?.map
      if (!rawSourceMap) {
      return input
      }
      const traced = new TraceMap(rawSourceMap as any)
      const pos = originalPositionFor(traced, {
      line: Number(line) - offset,
      // stacktrace's column is 1-indexed, but sourcemap's one is 0-indexed
      column: Number(column) - 1,
      })
      .
    • I think we can remove the code highlight feature for the following reasons:
      • Since this feature is mainly for LLMs, it doesn't help much.
      • We don't have that in other places.
      • @shijikijs/cli is huge and will made Vite's install size to be 1.5x.

@sapphi-red sapphi-red added p3-significant High priority enhancement (priority) feat: dev dev server labels Jul 28, 2025
@sapphi-red
Copy link
Member

We also need a new option server.streamBrowserErrorsToConsole so that this is an opt-in.

}

// error handler
middlewares.use(errorMiddleware(server, !!middlewareMode))
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This removal was accidental, will fix it!

@dominikg
Copy link
Contributor

dominikg commented Jul 29, 2025

ideally the location logged on the terminal console uses file:///path/to/src/some/cause.js:1:3 style so that users can click on it in their ide powered terminal open that directly. It might also work with relative ./src/some/cause.js:1:3 from cwd to avoid including the full path (esp useful if the user feeds this output to some llm thing or copy&pastes this somewhere public)

@dominikg
Copy link
Contributor

+1 to using the websocket, that would also get rid of the body-parser dependency.

try {
new Function('throw new Error(1)')()
} catch (e) {
// in Node 12, stack traces account for the function wrapper.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i don't think we have to support node12 here, our engine field asks for 20.19+

@sheremet-va
Copy link
Member

As a note, Vitest uses tinyhighlight that does the job well enough and is very small. I made it to support colors in error traces, but didn't want to have a huge library. So far, we had no complaints.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: dev dev server p3-significant High priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants